Adding support for .bzl file operations by mydatascience · Pull Request #1464 · bazelbuild/buildtools

mydatascience · 2026-05-08T17:34:17Z

Buildtools PR checklist

The code in this PR is covered by unit/integration tests.
I have tested these changes and provide testing instructions below.
I have either responded to, or resolved all Gemini comments on the PR.
I have read Google Eng Practices on Small Changes, this PR either follows these guidelines or the description provides reasoning for why they can not be followed.

Description

This PR extends edit label resolution to allow targeting .bzl files directly (for buildozer-style edits), e.g. //pkg:defs.bzl and //pkg:defs.bzl:rule. This makes it possible to apply edit transformations (like adding/removing entries from list attributes such as patches = [...]) to .bzl files, while still relying on buildifier-compatible formatting.

It also adds unit tests covering:

resolving .bzl targets via InterpretLabelForWorkspaceLocation
editing a patches list in a parsed .bzl file (add + remove) and validating formatted output

(optional) These changes were tested using the following steps

Run unit tests for the edited package:
go test ./edit
(Optional sanity) Run the specific new tests:
go test ./edit -run 'TestInterpretLabelForWorkspaceLocation_BzlFile|TestAddAndRemovePatchesInBzlFile'

gemini-code-assist

Code Review

This pull request extends the build tools to support editing .bzl files, enabling buildozer-style transformations on global variables. Key changes include updated label parsing for .bzl targets, improved global variable detection that excludes function-local assignments, and modifications to cmdAdd and cmdRemove. Feedback highlights critical logic errors in the new command handlers where variable-based rule checks were inverted and cmdRemove incorrectly used an addition function. Refactoring was also recommended to eliminate duplicated label parsing logic.

gemini-code-assist · 2026-05-08T17:36:14Z

 // The cmdXXX functions implement the various commands.

 func cmdAdd(opts *Options, env CmdEnvironment) (*build.File, error) {
+	if env.Rule != nil && env.Rule.Call != nil {


The condition env.Rule.Call != nil is incorrect for variable editing. According to the changes in expandTargets (lines 995-996), variable targets are represented by a dummy Rule where the Call field is nil. This check makes the logic for adding values to variables unreachable.

Suggested change

if env.Rule != nil && env.Rule.Call != nil {

if env.Rule != nil && env.Rule.Call == nil {

References

When modifying code, ensure that changes are consistent with previous behavior, especially if the previous behavior was intentional.

Signed-off-by: Vladimir Suvorov <suvorovv@google.com> Adding support for .bzl file operations Signed-off-by: Vladimir Suvorov <suvorovv@google.com> Adding support for .bzl file operations Signed-off-by: Vladimir Suvorov <suvorovv@google.com> Adding support for .bzl file operations Signed-off-by: Vladimir Suvorov <suvorovv@google.com> Fixing comments Signed-off-by: Vladimir Suvorov <suvorovv@google.com> Fixing comments Signed-off-by: Vladimir Suvorov <suvorovv@google.com> Fix tests Signed-off-by: Vladimir Suvorov <suvorovv@google.com>

mydatascience · 2026-05-08T19:07:01Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces support for editing global variables in .bzl files via buildozer. It includes updates to label resolution to handle .bzl files and refactors the Kind() method for more robust path extraction. Review feedback points out a risk of panics in existing commands due to the introduction of rules with nil call expressions and identifies limitations in the variable detection logic regarding tuple unpacking and loop variables.

gemini-code-assist · 2026-05-08T19:10:58Z

+			if as, ok := x.(*build.AssignExpr); ok {
+				if lhs, ok := as.LHS.(*build.Ident); ok {
+					vars[lhs.Name] = as
+				}
+			}


This implementation only detects global variables assigned via simple identifiers (e.g., VAR = ...). It will miss variables assigned via tuple unpacking (e.g., (A, B) = ...) or those defined as loop variables in top-level for statements. While this may be acceptable for common use cases, it is a limitation of the current variable detection logic.

Yes — that's a real limitation. Intended. Added to README

Signed-off-by: Vladimir Suvorov <suvorovv@google.com>

oreflow · 2026-07-06T12:39:55Z

PTAL at the buildkite errors before we spend time reviewing this

Signed-off-by: Vladimir Suvorov <suvorovv@google.com>

mydatascience · 2026-07-06T13:40:33Z

The previous fail was due to CI infra. Rerun is fine

oreflow

The .bzl expansion support LGTM as long as you add some documentation for that targets may be specified as such, but for the variable modifications we would require a strong reason to step back on that nodes are accessed and modified via the "name" attribute, if you have a specific case in mind it might even be better to support said case better via native rules instead of variables.

oreflow · 2026-07-06T15:51:32Z

Do you happen to have some further reasoning / requirements for the variable support?

It has been an intentional decision for the buildtools to identify targets using the "name" attribute since it generally simplifies BUILD file management as well as large scale changes across the google monorepo.

Use of variables also generally tends to have poor effects on build health, are not really aligned with https://bazel.build/build/style-guide#prefer-damp-build-files-over-dry, and is something we generally discourage.

So it would be interesting to learn what the specific need is, from what I can tell patches would generally be part of a target with a name as well?

Additionally the change to support variables seems to add some non-insignificant complexity across many of the commands.

oreflow · 2026-07-06T15:55:14Z

 buildozer 'copy testonly protolib' //pkg:py_protolib

-# Set two attributes in the same rule
+# Set two attributes in the same  rule


oreflow · 2026-07-06T16:21:44Z

+	bzlPart := rule[:idx+4]
+	rulePart = rule
+	if idx+4 < len(rule) && rule[idx+4] == ':' {
+		rulePart = rule[idx+5:]


I believe this is safe since all index offsets are in the string match with ".bzl", but just checking if you have tried this with unicode characters in the label to see if it might require rune based manipulation instead of char-index?

mydatascience requested a review from a team as a code owner May 8, 2026 17:34

mydatascience requested review from oreflow and removed request for a team May 8, 2026 17:34

gemini-code-assist Bot reviewed May 8, 2026

View reviewed changes

mydatascience force-pushed the bzl_support branch from 2772f32 to 6febf82 Compare May 8, 2026 18:52

mydatascience changed the title ~~[Draft]Adding support for .bzl file operations~~ Adding support for .bzl file operations May 8, 2026

gemini-code-assist Bot reviewed May 8, 2026

View reviewed changes

extention and fixes

bb44e4c

Signed-off-by: Vladimir Suvorov <suvorovv@google.com>

mydatascience force-pushed the bzl_support branch 2 times, most recently from 183af1b to bb44e4c Compare July 2, 2026 18:00

mydatascience added 2 commits July 2, 2026 22:04

Resolve rebase

1f4ca79

Signed-off-by: Vladimir Suvorov <suvorovv@google.com>

added to README and limitations

c2c5dc4

Signed-off-by: Vladimir Suvorov <suvorovv@google.com>

space

bb788b4

Signed-off-by: Vladimir Suvorov <suvorovv@google.com>

oreflow reviewed Jul 6, 2026

View reviewed changes

	if env.Rule != nil && env.Rule.Call != nil {
	if env.Rule != nil && env.Rule.Call == nil {

Uh oh!

Conversation

mydatascience commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Buildtools PR checklist

Description

(optional) These changes were tested using the following steps

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mydatascience commented May 8, 2026

/gemini review

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist Bot May 8, 2026

Choose a reason for hiding this comment

Uh oh!

mydatascience Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oreflow commented Jul 6, 2026

Uh oh!

mydatascience commented Jul 6, 2026

Uh oh!

oreflow left a comment

Choose a reason for hiding this comment

Uh oh!

oreflow Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

oreflow Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

oreflow Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mydatascience commented May 8, 2026 •

edited

Loading

mydatascience Jul 2, 2026 •

edited

Loading